fix(resync): publish off the reconcile pass's thread - #437
Merged
schickling merged 2 commits intoSep 4, 2026
Merged
Conversation
A reconcile pass could not complete while resync publication was making no progress. `install_live` and `deactivate` block on an acknowledgement from the resync worker thread, and that same thread ran every publication — which takes the shared catalog-authoring lock, re-resolves the catalog, and takes the recipient's stream lock. So a publication that was slow, refused and retried, or blocked on another process serialized the whole pass behind it, once per live seat. That is the property that let a terminal-refusal loop deny reconcile passes for two hours: the loop's only power was denying the pass that would have ended it. Publication now runs on its own thread. The worker captures a transition, hands it off, and applies the outcome when it returns, so it stays the only writer of carrier baselines and retry deadlines. One publication is outstanding per subscription at a time; a queued publication is dropped when its subscription is deactivated or removed by a refresh, so nothing is published to a seat the pass has already decided receives no events. Re-observing the carrier after an outcome is the single re-arm mechanism for both a retained refusal and a carrier that moved on while the publication was outstanding. This is the primary requirement from #431. Terminal-refusal classification is separate and still wanted: it removes the CPU burn and the unbounded retry, but it would leave this coupling intact for the next condition that starves pass completion. Refs #431 agent-identity: dev3.compoundingtech.st2.resync-lock.worker agent-persona: worker agent-supervisor: dev3.compoundingtech-lead agent-tool: OMP agent-tool-version: 18.1.2 agent-runtime: OMP 18.1.2 tooling-profile: dotfiles@7534055
The spec described the pass's acknowledged watch-set upsert and the retry replay, but not which thread publishes. That is the property a pass's ability to complete depends on, so it belongs in the spec rather than only in the code. Refs #431 agent-identity: dev3.compoundingtech.st2.resync-lock.worker agent-persona: worker agent-supervisor: dev3.compoundingtech-lead agent-tool: OMP agent-tool-version: 18.1.2 agent-runtime: OMP 18.1.2 tooling-profile: dotfiles@7534055
schickling-assistant
marked this pull request as ready for review
September 4, 2026 10:03
This was referenced Sep 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A reconcile pass could not complete while resync publication was making no progress.
ResyncSupervisor::install_liveanddeactivateblock on an acknowledgement from the resyncworker thread, once per live seat per pass. That same thread ran every publication, and a
publication is neither cheap nor bounded:
emit_admittedtakes the shared catalog-authoring lock,re-resolves the whole catalog through
discover_strict, and takes the recipient's stream lock. Soa publication that was slow, refused-and-retried, or blocked on another process serialized the
whole pass behind it.
That is the property which turned a noisy retry into a two-hour outage in #431. A resync refused
because its recipient is suspended is re-armed every 500 ms, and every attempt re-resolves the
catalog. The refusals had power only because they denied the reconcile pass that would have ended
them — the first completed pass tears the suspended seats' tasks down, and a refusal needs a live
task.
The coupling is observable directly. Holding the recipient's
resources/streams/resync/.lock—the exact flock the publication path takes — and then running a pass:
Goal
A reconcile pass completes whether or not resync publication is making progress, so no publication
condition can deny the pass that would resolve it.
Decisions
timeout. A timeout would leave the coupling in place and hide it behind a deadline; the
acknowledgement exists to order watch installation against launches, which is a read, not a
publication.
through the worker's own mailbox (
Msg::Emitted) instead of touching baselines or deadlines.Entry.in_flight). A reserved event identitymust not be handed off twice while the outcome that decides whether it is spent has not returned.
refresh, so nothing is published to a seat the pass has already decided receives no events. A
publication the emitter already started is left to finish: it was in flight while the
subscription was still active, and its outcome is discarded.
refusal and a carrier that moved on while the publication was outstanding. The first draft also
set
dirtyand a deadline in the refusal branch; that was redundant, and while it was presentit masked the re-observation clause under mutation (removing
poll_pathswas caught by notest). With the redundancy gone, that clause is caught by three.
not hold up supervisor teardown, and the emitter owns no state to hand back.
removes the CPU burn and the unbounded retry — but on its own it would leave this coupling
intact for the next condition that starves pass completion.
Verification
Fail-before / pass-after —
run::tests::reconcile_pass_completes_while_a_resync_publication_is_blockedholds the recipient's stream lock, confirms the publication has not completed, then runs a pass. A
watchdog releases the lock only if the pass fails to finish on its own, and the test asserts the
watchdog did not have to:
Mutation — each shipped clause removed one at a time, full resync + resync-pass test selection
run each time. No survivors:
a_flush_never_hands_off_a_subscription_whose_publication_is_outstandingdeactivation_drops_only_that_recipients_queued_publicationa_refresh_drops_a_queued_publication_for_a_subscription_it_removedfailed_tombstone_emit_retains_present_state_and_immutable_retry_snapshot+ 2The harness is self-checking: a run that does not compile, or that produces no test-result line, is
reported as UNRAN rather than as a surviving mutant. That guard earned itself — an earlier run of
the same matrix reported every mutant as surviving because
cargowas not on the PATH of theprocess that invoked it.
Gate — the exact test selection the Nix check runs (
--lib --bins --test discovery --test codex_hooks --test hooks --test run --test driver_expansion,--test-threads=1) is green locally:every target
ok, lib 680 passed / 0 failed.Suite —
cargo test -p st2 --lib: 680 passed, 0 failed.--test resync,--test resync_notify_chain,--test event_e2e,--test reconcile,--test doctor: all green.Every existing resync unit test still asserts exactly what it asserted before; they now drive the
worker and emitter halves in the order the loop runs them, through a
#[cfg(test)]helper.Production has one path.
docs/vrs/06-resync/spec.mdgains the property this depends on: where publication runs, oneoutstanding publication per subscription, and what happens to a queued or already-started
publication when a subscription goes away.
Complexity
One thread and one bounded queue. The queue exists because the two operations have genuinely
different liveness requirements: watch installation must answer a reconcile pass promptly, and
publication may block on another process. Merging them is what produced the defect. The alternative
— a mutex over carrier state released around each publication — needs the same outstanding/outcome
bookkeeping without the thread separation, so it is not simpler.
Concerns
delivery for every recipient behind it, though no longer any reconcile pass. This PR strictly
improves on the previous behaviour, where the same condition also stalled the passes; refusal
classification removes the case that made it likely.
publication was synchronous on the worker thread, so ordering was total. The window is now
bounded to a publication that was already in flight while the subscription was active. The
existing invariant test for that boundary
(
dead_resync_seat_is_deactivated_before_its_relaunch_blocks) still passes.question stays open there, and the pass rate should be measured separately rather than assumed
restored.
Friction & bottlenecks
INVARIANTS.mdnames three proofs intests/resource_profile_supervisor_e2e.rsthat no longerexist, so
tests/invariants.rs::qualified_proof_references_resolvefails. It fails identicallyon pristine
main(verified againstorigin/main'sINVARIANTS.md), and it is outside the Nixgate's test selection, which is why it has stayed red. The correct repair needs the author's
knowledge of which tests now prove that invariant, so it is not guessed at here.
tests/agent_publish.rsfails 9 of 22 withhost 'host' must declare exactly one root agent; found 0. Reproduced with this branch's files reverted to the merge-base content, so it ispre-existing and unrelated. Also outside the Nix gate.
The devshell's Nix wrapper prints
error (ignored): opening log file ... trace.jsonl: Permission deniedon every invocation. Harmless, but it is the first line of every build log.Overlaps PR Wake the supervisor promptly after catalog publication #433 on
INVARIANTS.mdandsrc/run.rs. Different regions — that PR adds acatalog-wakeup invariant row and touches
src/watch.rs; this one appends a row and adds a testinside
src/run.rs's existing test module. Whoever merges second resolves a trivialappend-vs-append conflict in the table.
Follow-ups
gets no retry deadline; the three permanently terminal refusals are dropped with one
diagnostic). Separate PR, same lane.
References
Refs #431
Posted on behalf of @schickling
agent_identitysessionagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile